-
-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[16.0][ADD] - sale_order_line_cancel #2357
base: 16.0
Are you sure you want to change the base?
Conversation
685f327
to
da21d81
Compare
cc/ @lmignon |
6418faf
to
f5d3431
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sbejaoui I tested the odoo native behavior and it seems that this addon could be simplified and only provide the functionality in a specific context. On a sale order line, you can edit the quantity even if the SO is validated... IMO this behavior is dangerous and should be forbidden. Hopefully, an option is available into the settings to prevent the modification of quantities when the sale order is confirmed... It's only in the context where the modification is no more possible that this functionality makes sens and should be activated. Moreover, I've the feeling that all the custom code written to cancel the moves could be removed (and the dependency on OCA/stock-logistics-workflow#1202). When you change the qty on a confirmed line, Odoo takes the necessary steps to reflect this change in the picking chain. The only thing Odoo doesn't do is to prevent the cancel of quantities that are partially processed. Instead, it generate the return picking to revert this outgoing moves... I prefer to prevent the cancellation of qties already partially processed than adding more work to do by opertors. This behaviour could be a configurable option..
Thanks @lmignon for your input, I understand your PoV, It's true that the change of the ordered quantity trigger the change/cancellation of the related moves. This feature is meant to simplify the process and make it done with a simple click. It also keep the information of the initial ordered quantity and store the canceled quantity. The difference between the two fields may be helpful for sale reporting. The dependency to OCA/stock-logistics-workflow#1202 is not a must. It's, IMO, a simpler way to get all sale line related moves instead of recursive loop. |
IMO, the functionality should only be available if the change of qty on confirmed sale order is activated.. |
f5d3431
to
db0f8d6
Compare
db0f8d6
to
74a1b1f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comment to add to the desciption
0f051b2
to
23f56a1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did many tests and it works quite well. It restricts the possibility of changing the quantity on the sale order line but at least the cancelation is working properly while in standard odoo you can end-up with a very weird reverse picking as decreasing the sale order line quantity generates a negative move that is supposed to be merged with the original move but if it can't be assigned to the same picking, it will create a reverse picking.
A few remarks that would make this module more complete:
- when you set the sales order back to quotation,
product_qty_canceled
on the lines should be reset to 0 - I would display the cancel button on a locked sales order as you expect auto-locking to be enabled and you allow canceling the lines
- When canceling the sales order, I would also write
product_qty_remains_to_deliver
inproduct_qty_canceled
- If you don't generate the backorder of the outgoing picking, the
product_qty_canceled
on the lines should also increase
Just a side note that this way of finding the right moves of the chain is not safe as moves can be merged. For example, a single pick move could serve multiple delivery moves. This specific use case is covered by the stock_available_to_promise_release module which, in case you cancel the outgoing move, will unrelease the outgoing move and while unreleasing it, will go through the picking chain up to the source picking and split any merged move. A solution would be that this module only cancel the outgoing move and have another module using the dependency to OCA/stock-logistics-workflow#1202 to propagate the cancelation. You can maybe add this remark in a ROADMAP file. |
23f56a1
to
a07abf4
Compare
a07abf4
to
34bf905
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the improvements
LGTM
34bf905
to
6940f11
Compare
This PR has the |
ping @sbejaoui :) |
Co-authored-by: Jacques-Etienne Baudoux <[email protected]>
thank you @sbejaoui 👍 @rousseldenis can yo check the PR ? |
Cancel remaining qty by running negative procurement orders to let odoo generate the appropriate operations
8585123
to
b5cd7f0
Compare
Co-authored-by: Jacques-Etienne Baudoux <[email protected]>
for rec in sale_moves: | ||
if rec.state != "cancel": | ||
continue | ||
rec.sale_line_id.product_qty_canceled = rec.product_uom_qty |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not correct for the kits (mrp)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should also ensure on sale.order.line that product_qty_canceled <= product_uom_qty - qty_delivered
|
||
def _action_cancel(self): | ||
orders = self.filtered(lambda s: s.state != "cancel") | ||
orders_with_picking = orders.filtered("picking_ids") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this is better as in sale_stock
they use qty_delivered_method
:
orders_with_picking = orders.filtered("picking_ids") | |
orders_with_picking = orders.filtered(lambda o: any(line.qty_delivered_method == 'stock_move' for line in o.order_line)): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should also support dropship & kits
in regards of https://github.com/odoo/odoo/blob/16.0/addons/sale_mrp/models/sale_order_line.py#L33
See acsone#4 |
This reverts commit b5cd7f0.
Allows to check the cancellation on sale order line level Include normal sales (not locked)
<field name="perm_create" eval="1" /> | ||
<field name="perm_write" eval="1" /> | ||
<field name="perm_unlink" eval="1" /> | ||
</record> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it possible to add entry for users in groups sales_team.group_sale_salesman ?
sale.order.line.cancel access own document29b4996
to
736014a
Compare
for line in self: | ||
if line._get_moves_to_cancel(): | ||
continue | ||
line.product_qty_canceled = line.qty_to_deliver |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The canceled quantity is not the same as the quantity to deliver because the quantity to deliver includes returned quantities.
For example, in an order of 10 units, if I deliver 8 and then cancel the remaining 2, and later my customer returns 2 units, I would expect the following values:
- Quantity to deliver: 4
- Quantity canceled: 2
_description = "Cancel Remaining Wizard" | ||
|
||
def _get_sale_order_line(self): | ||
active_id = self._context.get("active_id") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_context
is deprecated
active_id = self._context.get("active_id") | |
active_id = self.env.context.get("active_id") |
This module allows you to cancel the remaining quantity on sale order by adding
a dedicated action to sale lines. It also add two new fields to track canceled
and remaining to deliver quantities.